security: full audit fixes (2C, 4H, 7M, 5L) - #19

The shell handshake derived an ECDH session key between controller and
agent, with trust proved by each side's selfSig over
`publicKey + friendlyName + timestamp`. Critically, the signature was not
bound to the ECDH ephemeral keys or any transcript — so a relay that held
a shared secret on each leg (the explicit threat model per protocol-spec.md)
could decrypt the controller's encrypted identity on leg A, re-encrypt the
SAME valid-signed envelope on leg B, and the agent would verify the
signature, find the controller in its allow list, and derive a session key
from the relay-held shared secret. Full impersonation, no user interaction.

Fix: selfSig now covers a canonical transcript bound to the ECDH exchange:

  "amesh-shell-v1\n" ||
  publicKeyBase64    || "\n" ||
  deviceId           || "\n" ||
  friendlyName       || "\n" ||
  timestamp          || "\n" ||
  sha256(signerEphPub || verifierEphPub)

Each side signs using the ephemeral keys IT put on the wire and received,
and verifies using the ephemeral keys IT observed. A MITM sees different
peer ephemerals on each leg, so a signature captured from leg A no longer
verifies on leg B. The `amesh-shell-v1` domain prefix also prevents
cross-protocol reuse with the pairing selfSig format.

Applied in both packages/agent and packages/cli. `buildShellSigMessage` is
exported so tests can exercise it directly.

Regression tests:
 - signature bound to (A,B) does not verify against (A,C)
 - tampering deviceId / friendlyName / timestamp invalidates the sig
 - legacy pre-fix signature format is rejected (domain separator check)

Three related fixes in packages/relay/src/server.ts that all touch the
same file; split commits would require splitting a single file.

M1 — connectionCount double-decrement bypass of MAX_CONNECTIONS
  `open()` decremented on overflow rejection AND `close()` decremented
  again, so the counter drifted negative under repeated rejections and
  the > MAX_CONNECTIONS gate silently stopped firing. Fix: mark rejected
  sockets via `ws.data.rejected = true`, leave the decrement to `close()`,
  and skip `cleanupSocket` for rejected ones. Expose `maxConnections` as
  a createRelayServer option for testability.

H4 — bootstrap token single_use enforcement
  The payload claimed `single_use: true` but nothing ever tracked jtis,
  so a leaked token could pair N attacker-controlled targets within the
  TTL. Add a consumed-jti map with 25h TTL (covers MAX_TTL 24h + clock
  skew) and a 1M-entry cap. `handleBootstrapInit` rejects replayed jtis
  with `bootstrap_reject { error: "token_already_used" }` and burns the
  jti immediately on first init (fail-safe: even if downstream bootstrap
  fails, the token cannot be retried). Piggyback cleanup on the existing
  bootstrap watcher timer.

H1 — rate limiter X-Forwarded-For support
  `srv.requestIP(req)?.address` returned the load-balancer peer on Cloud
  Run / nginx / Cloudflare, collapsing all clients into one rate-limit
  bucket. Add `extractClientIp(req, srv, trustProxy)` that takes the
  left-most XFF entry (RFC 7239 originating client) when `trustProxy` is
  true, validates it via `isValidIp`, and falls back to the socket peer
  on malformed input or when trustProxy is false. Default reads
  `AMESH_TRUST_PROXY` env var; must be opt-in so directly-exposed relays
  don't honour spoofable headers. Cloud Run operators should set
  `AMESH_TRUST_PROXY=1`.

Regression tests:
  connection-limit.test.ts    2 tests — overflow + close + re-admit,
                                        burst of 5 rejections stays at
                                        correct counter
  bootstrap-single-use.test.ts 3 tests — same-jti replay rejected,
                                         distinct jtis work in parallel,
                                         jti burned on downstream failure
  forwarded-ip.test.ts       13 tests — IPv4/IPv6 validation, XFF
                                        left-most, malformed fallback,
                                        env-var default, don't-take-
                                        right-most regression

`validateBootstrapToken` previously only checked `exp <= now`. Four
holes closed:

  1. `iat` (not-before): a token with iat in the future — e.g. from a
     backdated issuer clock — silently extended the effective lifetime.
     Now rejected when `iat > now + 60s`.

  2. `header.alg`: never checked. Pinned to `ES256`. Defense-in-depth
     against future crypto swaps or alg-confusion attacks.

  3. `payload.scope`: never checked. Pinned to `peer:add` (the only
     scope currently defined). Future scopes must explicitly opt in
     rather than being silently honoured by old validators.

  4. `payload.single_use`: never checked. Must be `true`. The relay
     now enforces single-use via a jti registry (see H4), but the
     structural invariant is also checked here so tokens can't claim
     to be multi-use.

`typeof` guards added on `iat` / `exp` to reject structurally invalid
payloads before any crypto.

Error codes are now distinct:
  unsupported_token_alg, unsupported_token_scope,
  token_must_be_single_use, token_not_yet_valid, token_expired

Applied in packages/agent/src/bootstrap-token.ts and the cli mirror.

9 × 2 regression tests in bootstrap-token.test.ts covering each
rejection path individually plus a round-trip through the generator.

The encrypted-file backend auto-generated a 256-bit random passphrase and
wrote it into `~/.amesh/identity.json` next to the encrypted key file at
`~/.amesh/keys/<deviceId>.key.json`. Both files were mode 0o600 in the
same directory, so any attacker with filesystem read access got both in
the same step — defeating the point of the Argon2id + AES-256-GCM layer.

Fix:

  - New helpers in paths.ts (both agent and cli packages):
      getPassphrasePath()        — `~/.amesh/.passphrase`, or
                                   `AMESH_PASSPHRASE_FILE` override
      savePassphrase(pass)       — atomic tmp+rename, final mode 0o400
      deletePassphraseFile()     — idempotent cleanup for --force
      resolvePassphrase(identity) — env var → file → legacy identity.passphrase
                                    (auto-migrated with deprecation warning)

  - context.ts, agent.ts, shell-client.ts in both packages now use
    resolvePassphrase and persist the migration back to identity.json so
    legacy installs silently upgrade on next load.

  - commands/init.ts: prefers AUTH_MESH_PASSPHRASE env var so secrets can
    stay off disk entirely; otherwise auto-generates and writes to the
    dedicated file, NOT identity.json. On --force, stale passphrase files
    are cleared so a backend switch doesn't leave orphaned state.

  - identity.ts: `passphrase` field marked DEPRECATED; kept readable for
    backward compatibility while auto-migration runs.

  - sdk/bootstrap.ts: mirrors the same behaviour. Also completes the M6
    rollout for the SDK bootstrap path by calling validateTokenInvariants()
    before any network work (header.alg, scope, single_use, iat, exp).

Operator-visible change: on next startup, existing installs will print
one warning line when the legacy passphrase is migrated:

  [amesh] migrated legacy passphrase from identity.json to dedicated file.

9 × 2 regression tests in passphrase-location.test.ts cover resolution
priority, mode bits, atomic write, env-var / file override, auto-migration
from identity.passphrase, and the non-colocation invariant.

…-time agent store

M2 — SessionStore was unbounded, so a listen flood could OOM the relay.
  New DEFAULT_MAX_SESSIONS = 50_000 (tunable via createRelayServer options).
  When full, a last-ditch purge runs and, if still full, create() throws
  session_store_full. handleListen surfaces this as `relay_capacity` to
  distinguish from `otc_in_use` (OTC collision, retry-safe).

M3 — handleBootstrapWatch was last-write-wins with no auth and no rate
  limiting, so an attacker could continuously hijack legitimate watchers.
    - Reject claims from a DIFFERENT socket while a healthy watcher owns
      the jti (`jti_already_watched`).
    - Allow same-socket re-registration (reconnect idempotency).
    - Dedicated bootstrapWatchRateLimiter (10/min/IP) kept separate from
      the OTC limiter so heavy bootstrap traffic doesn't starve pairing.
    - Validate jti is a string of at most 128 characters.

L3 — AgentStore.register/matchAndGet compared public keys with `!==`.
  Pubkeys aren't secret, but the (deviceId, pubkey) tuple is the only gate
  between an enumerating attacker and "this pair is currently registered"
  side-channel info. Replaced with constantTimeStringEqual.

Regression tests:
  session-store.test.ts          5 unit tests — cap enforcement, purge on
                                                  miss, OTC-in-use distinct
                                                  from capacity
  session-cap-integration.test.ts 1 e2e test — `relay_capacity` wire code
  bootstrap-watcher-race.test.ts 5 tests    — first claim, hijack rejection,
                                              same-socket re-register,
                                              disconnect-reclaim, oversized
                                              jti

…connect

The handshake's createMessageReader installed a `message` listener on the
WebSocket and never removed it. During long shell sessions every encrypted
frame fired the reader's handler, growing its internal queue unbounded —
memory leak per session.

Separately, on ws.close() during an active shell the agent daemon scheduled
a reconnect but did NOT tear down the running bash process, idle timer, or
cipher. sessionActive stayed true, so no new session could be accepted
until the idle timeout fired (default 30 min). Orphaned bash processes
accumulated on every relay blip.

Fix:
  - createMessageReader now returns a dispose() that removes the listener,
    drains any pending waiter (rejecting with `reader_disposed`), and
    clears the queue. Idempotent via a `disposed` flag.
  - agent.ts/shell-client.ts dispose() the reader immediately after the
    handshake completes, and on all error paths.
  - agent.ts tracks an `activeSession` object { proc, cipher, idleCheck,
    messageHandler } in the outer scope. The ws.close handler calls
    teardownActiveSession('ws_disconnect') which kills the proc, clears
    the timer, closes the cipher, and resets sessionActive.
  - handleShellRequest removes its own encrypted-frame listener on exit.

Applied symmetrically in packages/agent/src and packages/cli/src.

Regression tests:
  message-reader-dispose.test.ts (both packages) — 5 tests each:
    listener removal, idempotent dispose, queue not growing post-dispose,
    pending read() rejects with reader_disposed, pre-dispose messages
    still consumable.

…d-object

authMeshVerify's previous getBody() fell back to JSON.stringify(req.body)
when an upstream parser like express.json() had already turned the body
into an object. This hashed a byte sequence that differed from what the
client signed:

  - Legitimate clients whose JSON formatting differed from V8's
    (whitespace, numeric precision, key order, duplicate-key handling)
    silently failed verification.
  - Any two byte sequences that parsed to the same object verified
    against the same signature — a latent relaxation of the BodyHash
    binding the spec defines.

Fix: new getRawBody(req, maxBytes) helper with a strict resolution order:
  1. req.rawBody (Buffer/Uint8Array) from a parser verify hook
  2. req.body as Buffer (express.raw())
  3. req.body as string (express.text())
  4. Stream-buffer ourselves with a configurable maxBodyBytes cap
     (default 1 MiB, Content-Length short-circuit)

A parsed-object req.body with NO rawBody is now a hard error: returns
500 body_parser_ordering_error rather than silently re-serializing.
Users MUST either mount authMeshVerify before body parsers, or pass
`verify: (req, _res, buf) => { req.rawBody = buf; }` to express.json().
Documented in docs/protocol-spec.md §8.

Also fixed sendError to stop flattening 5xx into {error: "unauthorized"}.
401 still flattens (prevents verification-state oracle attacks), but 400/
413/5xx return the specific code so misconfiguration is visible.

New VerifyOptions.maxBodyBytes with 1 MiB default.

Regression tests: middleware-rawbody.test.ts — 6 tests covering stream-
buffer path, non-canonical JSON whitespace preservation, parsed-object
rejection, verify-hook integration, and maxBodyBytes enforcement at
boundary.

Two crash-level bugs in the TPM backend made it non-functional for any
Linux deployment that successfully detected it:

1. tpm2_sign without --format=plain outputs a TPMT_SIGNATURE structured
   blob (2B scheme || 2B hash alg || 2B r-len || r || 2B s-len || s),
   not raw r||s. @noble/curves' p256.verify expects 64-byte raw r||s, so
   every signature from the TPM backend was rejected.

2. pemToRaw returned the full SubjectPublicKeyInfo DER (~91 bytes) from
   a PEM-encoded public key. The KeyStore interface contract is "33-byte
   compressed P-256 point". Every caller feeding the result into the
   canonical signing chain got garbage.

Fix:
  - sign() now passes --format=plain to tpm2_sign and falls back to
    parsing TPMT_SIGNATURE for tpm2-tools 4.x (Ubuntu 20.04) which lacks
    the flag. The fallback parser is a bounded, defensive field walker
    exported as parseTpmtSignature.
  - pemToRaw() now strips the PEM envelope, walks the SPKI DER via a
    bounded extractSec1PointFromSpki (accepts short- and limited long-
    form DER lengths), pulls out the 65-byte uncompressed SEC1 point
    from the BIT STRING, and compresses via p256.Point.fromHex().toBytes(true).
  - Both helpers exported so they can be tested in isolation (the TPM
    subprocess itself can't run on macOS CI).

Regression tests: tpm-parsers.test.ts — 10 tests including a full round-
trip through Node's crypto.generateKeyPairSync('ec', {namedCurve:
'prime256v1'}) → PEM → pemToRaw → valid P-256 compressed point, plus
TPMT_SIGNATURE truncation/scheme/length/r-overflow guards.

…hardening

L2 — parseAuthHeader was too permissive:
  - Silently accepted duplicate keys (v="1",v="2" → last wins)
  - No length caps on header or fields
  - Accepted unknown keys (forward-compat is supposed to go through `v=`)

Now rejects:
  - Headers longer than 1024 characters
  - Duplicate keys
  - Unknown keys
  - Per-field overflows: v≤8, ts≤16, nonce≤64, id≤128, sig≤256

Documented in docs/protocol-spec.md §7 (Authorization Header) with a
Parser Invariants subsection so conforming non-TS implementations match.

L4 — derToRaw in the macOS keychain driver walked der[i++] without any
bounds checks. The Swift helper binary is locally-signed so the trust
boundary is internal, but a buggy or tampered helper could have indexed
out of range and produced garbage output.

Now bounds-checks every field access, rejects long-form length encodings
(not valid for a P-256 ECDSA signature), enforces r/s length ≤ 32 bytes
after leading-zero strip, and throws specific errors instead of silently
producing corrupted output. Exported for testing.

Regression tests:
  header.test.ts — 5 new tests (duplicate keys, unknown keys, oversized
    header, per-field cap overflow, ts 16-char cap)
  der-parser.test.ts — 10 tests including a full round-trip through
    Node crypto.createSign('SHA256') → P256.verify, plus malformed-input
    guards (empty, wrong SEQUENCE tag, long-form length, truncated length,
    missing r tag, absurd r length, r overflow, missing s tag)

…ist HMAC

The HMAC input for allow_list.json was computed with plain JSON.stringify,
which preserves JavaScript object insertion order. That made the HMAC
brittle across code refactors, cross-runtime interop, and manual file
edits — deterministic today but a latent footgun.

Fix: new stableStringify that sorts object keys lexicographically at
every level, recurses into arrays in order, drops undefined fields to
match JSON semantics, and caps recursion at 32 levels as a sanity check.
The HMAC now binds to the CONTENT of the allow list, not the particular
object-construction order of the writer.

Backward compatibility: pre-L5 files sealed with plain JSON.stringify
are accepted on read via a legacy-canonical fallback, then automatically
re-sealed with the deterministic form on next write. Existing installs
silently migrate on first read.

Regression tests: allow-list.test.ts — 2 new tests:
  - HMAC survives key re-ordering on disk (proves sort-stability)
  - Legacy-canonical file loads successfully and is re-sealed with the
    new canonical, verified by a second read + fresh-HMAC assertion

… prefix

The controller ack signature used to cover `base64(pubkey) + jti` with no
delimiter between the two fields. Base64 pubkeys are a fixed 44 chars and
jtis are `bt_<hex>` so collision is unreachable in practice, but the
layout was fragile — any future format change to either field (e.g.
raw-encoded pubkeys, different jti namespace) could introduce ambiguity.

Now signed message is:

  "amesh-bootstrap-ack-v1\n" + base64(pubkey) + "\n" + jti

Explicit delimiters + amesh-bootstrap-ack-v1 domain prefix prevent cross-
protocol signature reuse if a future handshake variant ever signs similar
fields.

Any controller-side producer of bootstrap_ack messages must mirror the
new format. Noted in docs/remote-shell-spec.md and the protocol spec
error reference.

New:
  - docs/security-audit-2026-04.md — the full audit report, one section
    per finding with severity, attack scenario, fix, and regression test
    mapping. Covers all 2C + 4H + 7M + 5L fixes landed on this branch.
    Includes an "Operator actions required" checklist.

Protocol spec (docs/protocol-spec.md):
  - §3 tech stack table — encrypted-file row now documents the dedicated
    passphrase file (H2) and the AMESH_PASSPHRASE_FILE override.
  - §7 Authorization Header — new "Parser Invariants (L2)" subsection
    listing the strict rules conforming implementations must follow
    (duplicate keys rejected, length caps, unknown keys rejected).
  - §8 Verification Middleware — new "Middleware Ordering Contract (M5)"
    subsection documenting the raw-body resolution order and the
    body_parser_ordering_error 500 response. Shows the recommended
    `express.json({ verify: … })` pattern.
  - §9 Allow List — documents the deterministic stableStringify canonical
    JSON with recursive key sort (L5), and the legacy-canonical auto
    migration path.
  - §10 Relay — new subsections for:
      * H1 client IP extraction (AMESH_TRUST_PROXY, left-most XFF)
      * H4 single-use enforcement (25h consumed-jti set)
      * M3 bootstrap watcher race protection (jti_already_watched)
      * Relay rate-limiting numbers updated to match the new limiters
        and caps (M2 session cap, bootstrap_watch dedicated limiter).
  - §13 Security Considerations — new "Security audit — April 2026"
    table summarizing every fix and linking to the full writeup.
  - §14 Error Reference — new rows for 413 payload_too_large,
    500 body_parser_ordering_error, and a new table of bootstrap token
    validation errors (unsupported_token_alg/scope, token_must_be_
    single_use, token_not_yet_valid, token_expired, token_already_used).

Remote shell spec (docs/remote-shell-spec.md):
  - §7 ECDH Handshake — rewritten with a new §7.1 "Transcript-bound
    selfSig" section showing the exact canonical signed bytes for C1.
    The old "MITM protection without SAS" paragraph has been corrected
    — it was the exact logic flaw that caused C1 (claimed allow-list
    pubkey match was sufficient; in reality a MITM doesn't substitute
    permanent keys, it forwards real selfSig envelopes).
  - §8 Security Considerations — updated relay trust model to reference
    the transcript-bound selfSig, added "Frame cipher desync resistance
    (H3)" subsection.

CHANGELOG.md:
  - New [Unreleased] section listing all 18 findings by severity with
    a one-paragraph description each, plus an Operator Actions Required
    block covering AMESH_TRUST_PROXY, middleware ordering, and the
    passphrase migration log line.

Landpage:
  - docs/key-storage — rewritten Encrypted File Details section to
    show the new passphrase resolution order (env var → dedicated file
    → legacy migration), drop the "stores in identity.json" claim, and
    add an amber warning about filesystem-read adversaries.
  - docs/key-storage — added AMESH_PASSPHRASE_FILE to env var list.
  - docs/remote-shell — AUTH_MESH_PASSPHRASE reframed as a "preferred
    for production" option; added AMESH_PASSPHRASE_FILE entry.
  - docs/self-hosting — Cloud Run example now sets AMESH_TRUST_PROXY=1
    with an amber "Required" callout explaining why. Security section
    rewritten to mention transcript-bound shell MITM protection,
    separate bootstrap_watch rate limiter, single-use bootstrap tokens,
    session caps, and the XFF trust model.

CI caught a handful of lint issues introduced by the new regression tests
added in this branch:

  - middleware-rawbody.test.ts: unused `ServerResponse` import,
    `servers` declared with `let` but never reassigned.
  - tpm-parsers.test.ts and der-parser.test.ts: used `require('node:crypto')`
    (tripping @typescript-eslint/no-require-imports) and had an unused
    `privDer` variable in one round-trip test. Switched to top-level ESM
    imports of `generateKeyPairSync` + `createSign`.
  - packages/{agent,cli}/src/paths.ts: dead `// eslint-disable-next-line
    no-console` directives — the base config already allows console.warn.

No behavior changes. All 301 source-tree tests still pass, all 9 packages
lint clean.